Skip to content

Improve clarity of try-return-finally-return - #15677

Merged
matrixise merged 9 commits into
python:masterfrom
toonarmycaptain:patch-1
Sep 11, 2019
Merged

Improve clarity of try-return-finally-return#15677
matrixise merged 9 commits into
python:masterfrom
toonarmycaptain:patch-1

Conversation

@toonarmycaptain

@toonarmycaptain toonarmycaptain commented Sep 4, 2019

Copy link
Copy Markdown
Contributor

Clarify execution of try-return-finally-return case.
Per discussion https://discuss.python.org/t/pep-601-forbid-return-break-continue-breaking-out-of-finally

@toonarmycaptain
Improve clarity of try-return-finally-return
76133eb 
Clarify execution in try-return-finally-return case.
@toonarmycaptain
Update ACKS
aa6b90b 
Add my name to ACKS
@brandtbucher brandtbucher added the docs Documentation in the Doc dir label Sep 4, 2019
@toonarmycaptain
Update errors.rst
8d22940 
Remove trailing whitespace.
epicfaace
epicfaace reviewed Sep 10, 2019
View reviewed changes
Comment thread Doc/tutorial/errors.rst Outdated
Comment thread Doc/tutorial/errors.rst Outdated
@gpshead
gpshead requested a review from willingc September 10, 2019 15:14
willingc
willingc reviewed Sep 11, 2019
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @toonarmycaptain, thanks for the PR. I have left some suggested wording to help be more explicit and user friendly in tone for this tutorial.

Comment thread Doc/tutorial/errors.rst Outdated
when any other clause of the :keyword:`!try` statement is left via a
:keyword:`break`, :keyword:`continue` or :keyword:`return` statement. A more
complicated example::
:keyword:`!else` clause), it is re-raised after the :keyword:`!finally` clause has

@willingc willingc Sep 11, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a tutorial, I would recommend being explicit with a direct tone and minimal jargon. Some suggestions for change of this PR:

If a :keyword:finally clause is present, the :keyword:finally clause will execute as the last task before the :keyword:try statement completes. The :keyword:finally clause runs whether or not the :keyword:try statement produces an exception. The following points discuss more complex cases when an exception occurs:

  • If an exception occurs during execution of the :keyword:!try clause, the exception may be handled by an :keyword:except clause. In all cases, the exception is re-raised after the :keyword::!finally` clause has been executed.

  • An exception could occur during execution of an :keyword:!except or :keyword:!else clause. Again, the exception is re-raised after the :keyword::!finally` clause has been executed.

  • If the :keyword:!try statement reaches a :keyword:break, :keyword:continue or :keyword:return statement, the :keyword:finally clause will execute just prior to the :keyword:break, :keyword:continue or :keyword:return statement's execution.

  • If a :keyword:finally clause includes a :keyword:return statement, the :keyword:finally clause's :keyword:return statement will execute before, and instead of, the :keyword:return statement in a :keyword:try clause.

For example:

willingc
willingc requested changes Sep 11, 2019
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks and let me know if you need any clarification on the suggested changes.

@bedevere-bot

bedevere-bot commented Sep 11, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@toonarmycaptain

toonarmycaptain commented Sep 11, 2019

Copy link
Copy Markdown
Contributor Author

I believe I have made the requested changes; please review again.
Let me know if I need to modify the markdown syntax :)

@bedevere-bot

bedevere-bot commented Sep 11, 2019

Copy link
Copy Markdown

Thanks for making the requested changes!

@willingc: please review the changes made to this pull request.

willingc
willingc approved these changes Sep 11, 2019
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @toonarmycaptain. Just waiting for CI to pass.

@matrixise
matrixise merged commit 0cc2741 into python:master Sep 11, 2019
@miss-islington

miss-islington commented Sep 11, 2019

Copy link
Copy Markdown
Contributor

Thanks @toonarmycaptain for the PR, and @matrixise for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 11, 2019
@toonarmycaptain @miss-islington
Improve clarity of try-return-finally-return (pythonGH-15677)
ef7478e 
Clarify execution in try-return-finally-return case.
(cherry picked from commit 0cc2741)

Co-authored-by: toonarmycaptain <toonarmycaptain@hotmail.com>
@bedevere-bot

bedevere-bot commented Sep 11, 2019

Copy link
Copy Markdown

GH-15981 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 11, 2019
@toonarmycaptain @miss-islington
Improve clarity of try-return-finally-return (pythonGH-15677)
146daae 
Clarify execution in try-return-finally-return case.
(cherry picked from commit 0cc2741)

Co-authored-by: toonarmycaptain <toonarmycaptain@hotmail.com>
@bedevere-bot

bedevere-bot commented Sep 11, 2019

Copy link
Copy Markdown

GH-15982 is a backport of this pull request to the 3.7 branch.

matrixise pushed a commit that referenced this pull request Sep 11, 2019
@miss-islington @toonarmycaptain @matrixise
Improve clarity of try-return-finally-return (GH-15677) (GH-15982)
e4be7c8 
Clarify execution in try-return-finally-return case.
(cherry picked from commit 0cc2741)

Co-authored-by: toonarmycaptain <toonarmycaptain@hotmail.com>
matrixise pushed a commit that referenced this pull request Sep 11, 2019
@miss-islington @toonarmycaptain @matrixise
Improve clarity of try-return-finally-return (GH-15677) (GH-15981)
c0acc0e 
Clarify execution in try-return-finally-return case.
(cherry picked from commit 0cc2741)

Co-authored-by: toonarmycaptain <toonarmycaptain@hotmail.com>
@ammaraskar

ammaraskar commented Sep 12, 2019

Copy link
Copy Markdown
Member

https://bugs.python.org/issue38130 was filed a little bit ago stemming from this PR. Could the original author or one of the reviewers please take a look :)

DinoV pushed a commit to DinoV/cpython that referenced this pull request Sep 12, 2019
@toonarmycaptain @DinoV
Improve clarity of try-return-finally-return (pythonGH-15677)
1738f4d 
Clarify execution in try-return-finally-return case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willingc willingc willingc approved these changes
+1 more reviewer
@epicfaace epicfaace epicfaace left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@toonarmycaptain @bedevere-bot @miss-islington @ammaraskar @epicfaace @willingc @matrixise @the-knights-who-say-ni @brandtbucher